Skip to content

Conversation

@SannidhyaSah
Copy link
Collaborator

@SannidhyaSah SannidhyaSah commented May 28, 2025

Related GitHub Issue

Closes: #4065

Description

This PR implements OpenAI Compatible embedder support for codebase indexing, enabling users to connect to any OpenAI-compatible API endpoint (LiteLLM, LMStudio, Ollama, etc.) for generating embeddings.

Key implementation details:

  • New embedder class: OpenAiCompatibleEmbedder with intelligent batching and retry logic
  • Configuration management: Added base URL and API key settings with validation
  • UI integration: Dynamic provider selection with conditional input fields
  • Comprehensive testing: 20+ unit tests covering all functionality
  • Internationalization: Complete translations for 17 languages

Design choices:

  • Used OpenAI SDK for compatibility with all OpenAI-spec endpoints
  • Implemented batching with configurable sizes for optimal performance
  • Added exponential backoff retry logic for robust error handling
  • Followed existing patterns from other embedder implementations

Test Procedure

Automated Testing:

  1. Run pnpm test - All 20+ new unit tests pass
  2. Run pnpm lint - No linting errors
  3. Run pnpm check-types - All TypeScript types valid

Manual Testing:

  1. Open VSCode extension settings
  2. Navigate to Codebase Index settings
  3. Select "OpenAI Compatible" from provider dropdown
  4. Verify Base URL and API Key fields appear
  5. Test validation by leaving fields empty
  6. Configure with a valid OpenAI-compatible endpoint
  7. Verify embeddings are generated successfully

Type of Change

  • New Feature: Non-breaking change that adds functionality.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.
  • Yes, documentation updates are required. (This feature should be documented in user-facing docs explaining how to configure OpenAI-compatible endpoints for codebase indexing)

Additional Notes

This implementation supports all major OpenAI-compatible providers including:

  • LiteLLM proxy servers
  • LMStudio local servers
  • Ollama with OpenAI compatibility
  • Any other service implementing OpenAI's embeddings API spec

The feature is fully backward compatible and doesn't affect existing embedder configurations.


Important

Adds support for OpenAI-compatible embedders in codebase indexing with new embedder class, configuration, UI integration, and comprehensive testing.

  • Behavior:
    • Adds OpenAiCompatibleEmbedder class for OpenAI-compatible API endpoints with batching and retry logic.
    • Updates codebase-index.ts to include openai-compatible as an embedder provider.
    • UI changes in CodeIndexSettings.tsx to support dynamic provider selection.
  • Configuration:
    • Adds configuration options for base URL and API key in config-manager.ts.
    • Updates provider-settings.ts and global-settings.ts for new configuration keys.
  • Testing:
    • Adds 20+ unit tests in openai-compatible.test.ts and config-manager.test.ts.
    • Tests for configuration loading and validation in service-factory.test.ts.
  • Internationalization:
    • Updates translations in 17 language files for new UI elements.
  • Misc:
    • Adds OpenAiCompatibleEmbedder to service-factory.ts for embedder creation.
    • Updates embeddingModels.ts to include openai-compatible models.

This description was created by Ellipsis for 9755fd7. You can customize this summary. It will automatically update as commits are pushed.

- Implement OpenAiCompatibleEmbedder with batching and retry logic
- Add configuration support for base URL and API key
- Update UI with provider selection and input fields
- Add comprehensive test coverage
- Support for all OpenAI-compatible endpoints (LiteLLM, LMStudio, Ollama, etc.)
- Add internationalization for 17 languages
- Fix field count expectations (4 fields including Qdrant)
- Use specific test IDs for button selection
- Fix input handling with clear() before type()
- Use toHaveBeenLastCalledWith for better assertions
- Fix status text matching with regex pattern
- Remove unused waitFor import to fix ESLint error
- Fix test expectations to match actual component behavior for input fields
- Simplify provider selection test by removing complex mock interactions
- All CodeIndexSettings tests now pass (20/20)
@SannidhyaSah SannidhyaSah marked this pull request as ready for review May 28, 2025 06:45
@SannidhyaSah SannidhyaSah requested review from cte and mrubens as code owners May 28, 2025 06:45
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. documentation Improvements or additions to documentation enhancement New feature or request labels May 28, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label May 28, 2025
@hannesrudolph hannesrudolph moved this from Triage to PR [Needs Preliminary Review] in Roo Code Roadmap May 28, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels May 28, 2025
@daniel-lxs
Copy link
Member

daniel-lxs commented Jun 2, 2025

Hi @SannidhyaSah, thanks for this contribution! The ability to connect to various OpenAI-compatible endpoints is a great addition.

Regarding the introduction of the OpenAiCompatibleEmbedder class:

The new OpenAiCompatibleEmbedder shares a significant amount of its core logic (batching, retries, API client interaction) with the existing OpenAiEmbedder. Given this similarity, and considering that the system currently relies on a predefined list of models in EMBEDDING_MODEL_PROFILES rather than dynamically fetching model lists from the compatible endpoints, an alternative approach could be to enhance the existing OpenAiEmbedder.

We could potentially modify OpenAiEmbedder to accept an optional baseUrl in its constructor. If provided, this baseUrl would be used when instantiating the OpenAI client; otherwise, it would default to the standard OpenAI API URL.

This doesn't mean your implementation is wrong. However, it's an architectural consideration that might lead to a slightly more streamlined codebase in the long run.

What are your thoughts on this?

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jun 2, 2025
@SannidhyaSah
Copy link
Collaborator Author

Hi @SannidhyaSah, thanks for this contribution! The ability to connect to various OpenAI-compatible endpoints is a great addition.

Regarding the introduction of the OpenAiCompatibleEmbedder class:

The new OpenAiCompatibleEmbedder shares a significant amount of its core logic (batching, retries, API client interaction) with the existing OpenAiEmbedder. Given this similarity, and considering that the system currently relies on a predefined list of models in EMBEDDING_MODEL_PROFILES rather than dynamically fetching model lists from the compatible endpoints, an alternative approach could be to enhance the existing OpenAiEmbedder.

We could potentially modify OpenAiEmbedder to accept an optional baseUrl in its constructor. If provided, this baseUrl would be used when instantiating the OpenAI client; otherwise, it would default to the standard OpenAI API URL.

This approach might offer a few benefits:

* **Reduced Redundancy**: Avoids adding a new class that largely duplicates existing functionality.

* **Simplified Configuration**: The list of embedder providers in the settings would remain shorter, with the "OpenAI" provider gaining an optional "Base URL" field.

This doesn't mean your implementation is wrong. However, it's an architectural consideration that might lead to a slightly more streamlined codebase in the long run.

What are your thoughts on this?

@daniel-lxs I see the benefit of your proposed change in reducing the scope of modifications. My primary concern, though, is the potential for user confusion if we embed this within the OpenAI embedder. Given that the OpenAI compatible provider class functions as a universal proxy for other providers, not able to distinguishing between the two could become a UX nightmare.

That said, I've gone ahead and implemented your suggestion to see it in practice. If you're confident that the user confusion risk is acceptable, I'm ready to push these changes.

@daniel-lxs
Copy link
Member

daniel-lxs commented Jun 3, 2025

@SannidhyaSah
I think it would make sense to separate them in the case that the new provider allows the user to fetch or add new models, but since the models are exactly the same it might not be as confusing.

Here's one example:
image
image

Even though the provider is OpenRouter the user is able to set their own base URL.

Let me know what you think.

@SannidhyaSah
Copy link
Collaborator Author

SannidhyaSah commented Jun 3, 2025

@SannidhyaSah I think it would make sense to separate them in the case that the new provider allows the user to fetch or add new models, but since the models are exactly the same it might not be as confusing.

Here's one example: image image

Even though the provider is OpenRouter the user is able to set their own base URL.

Let me know what you think.

@daniel-lxs I appreciate your insight on how identical models might reduce immediate confusion, and I certainly see the logic in that perspective.

However, my concern about potential user confusion in the long run remains, especially given the broader and evolving landscape of embedding models and the distinct, crucial role our OpenAICompatible provider already plays. As you mentioned this will come in handy when new provider allows the user to fetch or add new models , but alredy exist .. For instance, other providers like Mistral AI not only offer their own embedding models, as seen here: https://docs.mistral.ai/capabilities/embeddings/code_embeddings/, but they also frequently provide free access to their APIs, which is often a strong preference for many users.

Beyond that, our OpenAICompatible endpoints are incredibly valuable for users connecting to local model hosting solutions like LM Studio and Ollama. For those using Open Web UI with Ollama to host their models on a VM, this integration is a real lifesaver. This makes the OpenAICompatible method a truly essential way for users to interact with a wide array of providers and their self-hosted models.

Looking further ahead, maintaining this as a completely separate class for OpenAICompatible providers will actually significantly reduce the need to add any other new embedder classes in the future, as it's designed to be a flexible gateway. Furthermore, having it as a distinct class offers a critical security and stability benefit: if we ever need to adapt or modify something specifically for these external or compatible models, we only need to work within the OpenAICompatible class. This approach keeps the main OpenAIEmbedder class undisturbed, ensuring it remains the most secure and stable option for direct OpenAI integrations.

Therefore, maintaining this clear architectural separation between the OpenAI embedder and the broader OpenAICompatible provider helps ensure users don't get confused as they navigate various embedding options, both now and in the future, while also streamlining our maintenance and future development efforts.

I can add a logic to fetch the models dynamically . That will resolve most of the future problems we might face here.

Let me know your thoughts so that I can proceed accordingly.

@daniel-lxs
Copy link
Member

@SannidhyaSah
Do you know of any OpenAI compatible provider that also serves the OpenAI text-embedding-ada-002, text-embedding-3-small and text-embedding-3-large?

I just don't think adding another provider to just have these same models in the list would be too useful for the users.

Let me know what you think.

@SannidhyaSah
Copy link
Collaborator Author

oh i see the confusion here .
@daniel-lxs what if i remove the hardcoded models from this new provider class completely . That was an oversight on my side . What if we have a function to fetch the dynamically instead here. Just like it's done in the normal Open AI compatible providers for chat completions. Does it solve the problem ? What do you think ?

@daniel-lxs
Copy link
Member

@SannidhyaSah Yes! That was my idea, if we can somehow dynamically list them then it totally makes sense to have a separate provider for it.

@mrubens
Copy link
Collaborator

mrubens commented Jun 3, 2025

Yeah what we do in the OpenAI Compatible provider in general is to try to list the models using /v1/models, but to also let the user enter their own model name in case that listing endpoint doesn't work.

For what it's worth one use case I heard about recently is people who want/need to use LiteLLM to proxy their model calls. In this case it's the same models, but through a proxy to meet their company's requirements.

SannidhyaSah and others added 13 commits June 4, 2025 07:25
- Add manual model ID and embedding dimension configuration
- Enable custom model input via text field in settings UI
- Add modelDimension parameter to OpenAiCompatibleEmbedder
- Update configuration management to persist dimension setting
- Prioritize manual dimension over hardcoded model profiles
- Add comprehensive test coverage for new functionality

This allows users to specify any custom embedding model and its
dimension for OpenAI-compatible providers, removing dependency
on hardcoded model profiles.
…eEmbedder

- Remove modelDimension property and constructor parameter from OpenAiCompatibleEmbedder class
- Update ServiceFactory to not pass dimension to embedder constructor
- Update tests to match new constructor signature
- The dimension is still used for QdrantVectorStore configuration
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@adamhill
Copy link
Contributor

adamhill commented Jun 4, 2025

Testing on LM Studio

OpenAI Provider for Embeddings Testing.pdf

Apologies for the PDF, I am still trying to figure out getting MD files out of Obsidian easily

@daniel-lxs
Copy link
Member

daniel-lxs commented Jun 4, 2025

Hey @adamhill,

We are using the OpenAI client to create the embeddings, it seems like LM Studio is not 100% compatible if it's failing like this. I tested with OpenAI and it works without any issues.

In that case is probably better to wait for the LM Studio provider.

@daniel-lxs daniel-lxs added the lgtm This PR has been approved by a maintainer label Jun 4, 2025
@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Review] in Roo Code Roadmap Jun 4, 2025
@adamhill
Copy link
Contributor

adamhill commented Jun 4, 2025

FYI. Per the OpenAPI specs v1/embeddings/ is the correct URL, no /models/ in-between as shown in the LMS developer console coming from Roo. Am I misinterpreting the specs?

https://platform.openai.com/docs/api-reference/embeddings/create

Edit: cURL works against LMS

  • no /models/ in the URL
  • normal model ID
curl http://10.5.1.251:1234/v1/embeddings \
 -H "Authorization: Bearer $OPENAI_API_KEY" \
 -H "Content-Type: application/json" \
 -d '{
   "input": "The food was delicious and the waiter...",
   "model": "nomic-embed-code",
   "encoding_format": "float"
 }'

{
 "object": "list",
 "data": [
   {
     "object": "embedding",
     "embedding": [
       0.02144342102110386,
       -0.010139767080545425,
       -0.01469159685075283,
       
       ....

LMS Logs

2025-06-04 15:23:15  [INFO] 
Received request to embed: The food was delicious and the waiter...
2025-06-04 15:23:15  [INFO] 
Returning embeddings (not shown in logs)

@mrubens mrubens merged commit a80795b into RooCodeInc:main Jun 4, 2025
13 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 4, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 4, 2025
@daniel-lxs
Copy link
Member

@adamhill
We are not calling any endpoints with anything other than the OpenAI client itself, and it works when I tested it with OpenAI.

I'm not sure what's going on with LM Studio but we basically are just using the official OpenAI client, this PR only passes a different base URL.

@SannidhyaSah SannidhyaSah deleted the feat/openai-compatible-embedder branch June 8, 2025 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request lgtm This PR has been approved by a maintainer PR - Needs Review size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add OpenAI Compatible embedder support for codebase indexing

5 participants